fix(native): seed typeMap entries for let/var object-literal methods#1555
Conversation
…1551) match_js_type_map's variable_declarator case now calls seed_objlit_type_map_entries for all declaration kinds (const/let/var) when at non-function scope. This mirrors WASM's handleVarDeclaratorTypeMap which has no isConst guard for object properties. For const, extract_object_literal_functions already seeds these entries; dedup_type_map collapses duplicates at equal confidence so existing behavior is unchanged.
Greptile SummaryFixes issue #1551 by seeding composite typeMap entries for
Confidence Score: 5/5Safe to merge — the fix is well-scoped, const paths are unchanged (dedup collapses any new duplicate typeMap entries), and the native Rust tests verify all three dispatch paths end-to-end including call-edge creation. All three resolution paths (method shorthand, shorthand property, pair+arrow/function) are correctly implemented for let/var parity with const. The deferred qualified-definition pass now covers all declaration kinds without producing duplicates for const. Scope guards prevent function-local object literals from polluting module-level typeMap. Native unit tests assert type_name correctness and call-edge creation for each case. The only gap is the WASM test asserting only key presence rather than full resolution, which the native tests already cover. tests/parsers/javascript.test.ts — the new WASM test could assert type_name and call-edge creation to match the thoroughness of the native tests. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["variable_declarator node\nlet/var/const obj = {...}"] --> B{"value is object?"}
B -- No --> Z[skip]
B -- Yes --> C{"inside function scope?"}
C -- Yes --> Z
C -- No --> D["seed_objlit_type_map_entries\nfor ALL decl kinds"]
D --> D1["shorthand: typeMap obj.e4 → e4"]
D --> D2["pair+identifier: typeMap routes.get → handler"]
D --> D3["pair+arrow/fn: typeMap api.save → api.save"]
D --> D4["method_def: typeMap obj.f → f (bare)"]
E["match_js_objlit_qualified_method_defs\ndeferred second pass"] --> F{"method_definition?\nall decl kinds"}
F -- Yes --> G["push qualified Definition: obj.f"]
E --> H{"pair + is_const=false?"}
H -- "Yes: let/var only" --> I["push qualified Definition: api.save"]
J["extract_object_literal_functions\nconst only"] --> K["push qualified Definition + typeMap\nfor pair+arrow/fn"]
J --> L["typeMap only for method_definition"]
M["dedup_type_map"] --> N["collapses duplicate entries\nat equal confidence\nconst gets entries from both paths"]
Reviews (5): Last reviewed commit: "fix(native): emit qualified definitions ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "method_definition" => { | ||
| // Method shorthand: `obj = { baz() {} }` → typeMap['obj.baz'] = 'obj.baz' | ||
| // Mirrors WASM: setTypeMapEntry(typeMap, qualifiedKey, qualifiedKey, 0.85). | ||
| let Some(method_name) = resolve_method_def_name(&child, source) else { continue }; | ||
| let qualified = format!("{}.{}", var_name, method_name); | ||
| symbols.type_map.push(TypeMapEntry { | ||
| name: qualified.clone(), | ||
| type_name: qualified, | ||
| confidence: 0.85, | ||
| }); |
There was a problem hiding this comment.
type_name mismatch breaks call resolution for let/var method shorthand
For let/var declarations, match_js_objlit_qualified_method_defs is still guarded by is_const (line 665), so no qualified definition (e.g. "obj.baz") is ever created. Only the bare definition "baz" exists (from handle_method_def). When a caller resolves obj.baz(), the resolver looks up typeMap["obj.baz"] → "obj.baz", then tries to find a definition named "obj.baz" — which doesn't exist for let/var. The call edge is never created, leaving issue #1551 effectively unfixed for the method-shorthand case.
extract_object_literal_functions (which runs for const) uses type_name: method_name (bare name) precisely to point at the bare definition, and the inline comment there says "points to the bare-name definition so the two-step accessor dispatch resolves via the bare node." The method_definition arm here should use type_name: method_name.to_string() instead of type_name: qualified, to consistently resolve against the bare definition that actually exists for let/var method shorthand.
There was a problem hiding this comment.
Fixed — changed type_name from the qualified name to the bare method name, matching the const path in extract_object_literal_functions. For let/var, no qualified definition exists (match_js_objlit_qualified_method_defs is const-only), so pointing at 'obj.baz' would leave resolution broken.
| #[test] | ||
| fn let_var_objlit_seeds_type_map_entries() { | ||
| // Method shorthand: `let obj = { f() {} }` → typeMap['obj.f'] present | ||
| let s_let_method = parse_js( | ||
| "let obj = { f() { return 1; } };\n\ | ||
| obj.f();", | ||
| ); | ||
| let tm = s_let_method.type_map.iter().find(|e| e.name == "obj.f"); | ||
| assert!(tm.is_some(), "let obj method: typeMap 'obj.f' missing; got: {:?}", | ||
| s_let_method.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()); | ||
|
|
||
| // Shorthand property: `var obj = { e4 }` → typeMap['obj.e4'] = 'e4' | ||
| let s_var_shorthand = parse_js( | ||
| "function e4() {}\n\ | ||
| var obj = { e4 };", | ||
| ); | ||
| let tm2 = s_var_shorthand.type_map.iter().find(|e| e.name == "obj.e4"); | ||
| assert!(tm2.is_some(), "var obj shorthand: typeMap 'obj.e4' missing; got: {:?}", | ||
| s_var_shorthand.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()); | ||
| assert_eq!(tm2.unwrap().type_name, "e4"); | ||
|
|
||
| // Pair with identifier value: `var routes = { get: handler }` → typeMap['routes.get'] = 'handler' | ||
| let s_var_pair = parse_js( | ||
| "function handler() {}\n\ | ||
| var routes = { get: handler };", | ||
| ); | ||
| let tm3 = s_var_pair.type_map.iter().find(|e| e.name == "routes.get"); | ||
| assert!(tm3.is_some(), "var routes pair: typeMap 'routes.get' missing; got: {:?}", | ||
| s_var_pair.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()); | ||
| assert_eq!(tm3.unwrap().type_name, "handler"); | ||
|
|
||
| // Pair with arrow value: `let api = { save: () => {} }` → typeMap['api.save'] = 'api.save' | ||
| let s_let_arrow = parse_js( | ||
| "let api = { save: () => {} };\n\ | ||
| api.save();", | ||
| ); | ||
| let tm4 = s_let_arrow.type_map.iter().find(|e| e.name == "api.save"); | ||
| assert!(tm4.is_some(), "let api arrow: typeMap 'api.save' missing; got: {:?}", | ||
| s_let_arrow.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()); | ||
| assert_eq!(tm4.unwrap().type_name, "api.save"); | ||
|
|
||
| // Scope guard: object literal inside a function body must NOT seed module-level typeMap. | ||
| let s_scoped = parse_js( | ||
| "function init() {\n\ | ||
| let local = { run() {} };\n\ | ||
| local.run();\n\ | ||
| }", | ||
| ); | ||
| assert!( | ||
| s_scoped.type_map.iter().all(|e| e.name != "local.run"), | ||
| "function-scoped let obj must not pollute typeMap; got: {:?}", | ||
| s_scoped.type_map.iter().map(|e| &e.name).collect::<Vec<_>>() | ||
| ); | ||
| } | ||
|
|
||
| /// Phase 8.3e: call receiver is correctly recorded for obj.f() inside defProp body. | ||
| #[test] | ||
| fn call_receiver_for_define_property() { |
There was a problem hiding this comment.
The five new unit tests assert that typeMap["obj.f"] is Some, but none verify that a call to obj.f() actually produces a call edge (symbols.calls). A typeMap entry with a type_name pointing to a nonexistent definition silently passes all these assertions while leaving resolution broken. Adding at least one assertion like assert!(s_let_method.calls.iter().any(|c| c.callee == "obj.f" || c.callee == "f")) would catch the type_name mismatch described above and prevent future regressions in call resolution for let/var object-literal methods.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed — added assertions for both type_name value ('f', not 'obj.f') and call-edge creation (calls contains name='f' with receiver='obj'). The type_name assertion directly catches the regression fixed in the sibling comment.
Codegraph Impact Analysis4 functions changed → 1 callers affected across 1 files
|
…entries For let/var object-literal declarations, match_js_objlit_qualified_method_defs is const-only, so no qualified definition (e.g. 'obj.baz') exists. Pointing typeMap['obj.baz'] → 'obj.baz' left resolution broken because the resolver found no matching node. Fix: use the bare method name as type_name, mirroring extract_object_literal_functions for const. Also strengthen the regression test with type_name value assertion and end-to-end call-edge creation check.
| match val_n.kind() { | ||
| "arrow_function" | "function_expression" | "function" => { | ||
| // Store qualified name as value so the resolver finds the qualified def. | ||
| // Mirrors WASM: setTypeMapEntry(typeMap, qualifiedKey, qualifiedKey, 0.85). | ||
| symbols.type_map.push(TypeMapEntry { | ||
| name: qualified.clone(), | ||
| type_name: qualified, | ||
| confidence: 0.85, | ||
| }); |
There was a problem hiding this comment.
pair+function arm seeds typeMap but omits the Definition, breaking call resolution
For let api = { save: () => {} }, this arm seeds TypeMapEntry { name: "api.save", type_name: "api.save" } but never creates the matching Definition { name: "api.save", ... }. The const path in extract_object_literal_functions (line 519–534) creates both the definition and the typeMap entry. When the resolver follows typeMap["api.save"] → "api.save" and then searches symbols.definitions for a node named "api.save", it finds nothing and drops the call edge.
Before this PR typeMap["api.save"] didn't exist, so api.save() took a direct-call path. After this PR the resolver enters the two-step dispatch and dead-ends on the missing definition, which is a regression for let/var objects with arrow/function-expression property values. The test s_let_arrow includes api.save() in the source but has no call-edge assertion, masking the broken state — unlike s_let_method which was correctly upgraded to assert call-edge creation.
There was a problem hiding this comment.
Fixed — extended match_js_objlit_qualified_method_defs to emit a qualified definition for let/var pair+arrow/function values (commit 0320b97). For const, extract_object_literal_functions already creates the definition, so const pairs are excluded to avoid duplicates. The test now asserts both that the qualified definition 'api.save' exists in definitions and that api.save() produces a call edge with receiver='api'.
| "arrow_function" | "function_expression" | "function" => { | ||
| // Store qualified name as value so the resolver finds the qualified def. | ||
| // Mirrors WASM: setTypeMapEntry(typeMap, qualifiedKey, qualifiedKey, 0.85). | ||
| symbols.type_map.push(TypeMapEntry { | ||
| name: qualified.clone(), | ||
| type_name: qualified, | ||
| confidence: 0.85, | ||
| }); | ||
| } |
There was a problem hiding this comment.
pair + inline function value seeds an unresolvable typeMap entry for let/var
seed_objlit_type_map_entries sets typeMap["api.save"] = "api.save" (qualified → qualified), but extract_object_literal_functions — the only place that creates a qualified definition like api.save(function) — is called exclusively from the is_const-gated branch at line 1312. For let/var, no definition named "api.save" is ever pushed to symbols.definitions. The resolver will find the typeMap entry, look up "api.save" in definitions, find nothing, and drop the call edge. The s_let_arrow test (pair + arrow) checks only that the typeMap key exists, not that api.save() produces a call edge, so this gap is invisible to the test suite. Either extend match_js_objlit_qualified_method_defs (or a new analog) to emit definitions for let/var arrow/function pair values, or align type_name with the bare strategy used by method shorthand to point at something that actually exists.
There was a problem hiding this comment.
Fixed — same root cause as the sibling comment. Extended match_js_objlit_qualified_method_defs (commit 0320b97) to emit qualified definitions for let/var pair+arrow/function values. The typeMap entry 'api.save' → 'api.save' (qualified→qualified) now resolves correctly because the matching 'api.save' definition is created by the deferred second-pass walker. The s_let_arrow test now asserts definition existence and end-to-end call-edge creation.
…on object-literal methods
For 'let api = { save: () => {} }', seed_objlit_type_map_entries correctly seeds
typeMap['api.save'] = 'api.save' (qualified), but no matching definition named
'api.save' existed for let/var — the resolver followed the typeMap entry and
dead-ended on a missing node.
Fix: extend match_js_objlit_qualified_method_defs to cover all declaration kinds
for method_definition (was const-only) and to emit qualified definitions for
let/var pair+arrow/function values (const pairs are handled by
extract_object_literal_functions, so they are excluded to avoid duplicates).
Tests: add definition-existence and call-edge assertions for the s_let_arrow case,
matching the pattern already established for s_let_method.
1 participant
Summary
match_js_objlit_qualified_method_defsbailed early for non-constdeclarations (the issue's bail-early at line 585 in javascript.rs). However, the deeper root cause was thatmatch_js_type_map'svariable_declaratorcase did not handle object-literal property typeMap seeding for any declaration kind — it was entirely done byextract_object_literal_functionswhich is only called from theconst-gated branch.seed_objlit_type_map_entries— a new helper that seeds composite typeMap keys from object literals for all declaration kinds (const,let,var) at non-function scope. This mirrors WASM'shandleVarDeclaratorTypeMapwhich has noisConstguard for the object literal section.match_js_type_map'svariable_declaratorcase. Forconst,extract_object_literal_functionsalready seeds identical entries;dedup_type_mapcollapses duplicates at equal confidence so existingconstbehavior is unchanged.letmethod shorthand.Closes #1551